Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MOD: new grouped tagging system for train protection #118

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

davide84
Copy link
Contributor

@davide84 davide84 commented Dec 4, 2023

This PR aims at implementing the solution outlined in this discussion
https://wiki.openstreetmap.org/wiki/Proposal:Railway:train_protection

and in general aims at reducing the complexity needed to add new train protection systems to ORM, namely

  • only 2 files will have to be updated instead of 3
  • functions will use only 1 train protection parameter instead of one parameters per train protection system

What I introduce in this PR is backward-compatible and will allow for a gradual update, first of tags and then of ORM. Of course I'm willing to do some work :)

I added in OSM the new tag describing SCMT on one Italian line in the neighboorhood of Turin; here you can see the new tag rendered in pink and the existing tags rendered in blue or green.

Screenshot from 2023-12-04 12-02-36

If this PR is approve I propose that

  • Italy is retagged to establish the method; I volunteer to do it, I started the SCMT tagging one year ago, it will be quick and not break anything since the current tag is very recent
  • all new systems, most of which are not tagged, are added with the new method
  • we gradually move to the new method the existing tags, starting from the more localized ones (e.g. ASFA)

@besentv
Copy link
Contributor

besentv commented Dec 7, 2023

I'd suggest having a function that converts the old tags to the new one and immediately change the "railway_train_protection_rank" and "railway_train_protection_rendered" functions to completely use the new system, so if everything got re-tagged, we can just easily remove the helper function.

@davide84
Copy link
Contributor Author

davide84 commented Dec 7, 2023

Hi!
I am not sure we really need the "railway_train_protection_rank", I left it for compatibility in the transition phase. If it could be removed, and I think so because "rank" is only used for ordering the SELECT results, then "railway_train_protection_rendered" would basically coincide with the proposed converting function.
I'll have a look into it.

EDIT: yes, we can do without. "railway_train_protection_rendered" is now the only place to contain the handling of tags.

@besentv
Copy link
Contributor

besentv commented Dec 7, 2023

I meant having railway_train_protection_rendered to already contain the code for all new tags and for backwards compatibility have another function to convert the old tags to the new one.

@DerDakon
Copy link
Contributor

DerDakon commented Dec 9, 2023

I generally agree, but…

We must finish the definition of the tagging first. I very much like the idea to just "ban" the addition of new systems using the old scheme. Technically, i.e. adapting the code, you can try everything out. But please don't change other peoples tagging just for experiments. Doing it on your own is IMHO fine, although I would prefer that you keep the old tagging as well until things are approved.

While I have been slacking for the proposal I would welcome any help getting it up to vote. But the contents and transitions should be mostly discussed on the wiki, and use GH only for the technical adaption.

@besentv
Copy link
Contributor

besentv commented Dec 9, 2023

That's why I suggested the conversion function. So the transition in ORM is seamless regardless of when the new tagging is applied.

@AwFi0919
Copy link

Anyways, I volunteer myself too for retagging all Italy if needed.
Maybe is asking too much but why not dotted lines (like multiple gauges) if there are more than one train protection systems?

@davide84
Copy link
Contributor Author

davide84 commented Feb 25, 2024

Hi!

Just a few comments, since apparently the development ran again into the issue that this PR is designed to solve.

We must finish the definition of the tagging first.

Could we limit the scope on "define a new tag for the signalling systems currently implemented in ORM"? Otherwise it's very much a neverending task. Who is actively working on the definition, anyways?

I very much like the idea to just "ban" the addition of new systems using the old scheme.

My feeling is that what is displayed in ORM gets tagged, the rest does not exist. See SCMT, things start to get tagged when someone does a PR here, not when something is added to the Wiki. Sorry but if that was the real process we would have all the world tagged.

Technically, i.e. adapting the code, you can try everything out. But please don't change other peoples tagging just for experiments.

In OSM I have changed nothing, I have ADDED tags for this test. I don't see a problem for ORM to specify what can be displayed or not, people will still be able to tag whatever they want, how they want. See for example construction:railway and railway:construction, duplicates are already everywhere, as a rendering software you are free choose what to display.

I would prefer that you keep the old tagging as well until things are approved.

Sorry if I ask, but I completely don't understand how this approval works, namely who exactly is involved and where the process takes place. I tried to jump in, the wiki discussion is half dead, here the activity is minimal, is there a mailing list or something? I'm really worried that we may wait for something that is not coming.

What I have seen so far is that something is not tagged, one starts tagging it and proposes a PR here, and suddenly things appear on maps and others start tagging because they see it working. Before my PR on SCMT the tagging coverage was a couple of lines across all Italy, after the PR it was the entire country in a matter of a few months (and it was not just me).
I think this "can do" approach could work for the new tagging, as long as we don't delete any other tag in OSM.

@STemplar
Copy link

My feeling is that what is displayed in ORM gets tagged, the rest does not exist. See SCMT, things start to get tagged when someone does a PR here, not when something is added to the Wiki. Sorry but if that was the real process we would have all the world tagged.

Sorry if I ask, but I completely don't understand how this approval works, namely who exactly is involved and where the process takes place. I tried to jump in, the wiki discussion is half dead, here the activity is minimal, is there a mailing list or something? I'm really worried that we may wait for something that is not coming.

What I have seen so far is that something is not tagged, one starts tagging it and proposes a PR here, and suddenly things appear on maps and others start tagging because they see it working. Before my PR on SCMT the tagging coverage was a couple of lines across all Italy, after the PR it was the entire country in a matter of a few months (and it was not just me). I think this "can do" approach could work for the new tagging, as long as we don't delete any other tag in OSM.

Agree with @davide84 here, add more Train Protection tags and the rest wil come eventually. I want to add more but the color-scheme will be a mess.

So one of the first things needed will be a regional / continental grouped lists of (national) train protection systems with accompanying colors.

@Bauer33333
Copy link

Sorry if I ask, but I completely don't understand how this approval works, namely who exactly is involved and where the process takes place. I tried to jump in, the wiki discussion is half dead, here the activity is minimal, is there a mailing list or something? I'm really worried that we may wait for something that is not coming.

The current status of the proposal is just a draft, so not intended to be seen yet. Once he has finished writing the proposal wiki page he can officially propose it and publish it on the forum and mailing list so people see it and will comment.

You can read more about the process in the wiki.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants